Skip to content

fix: setup cache on tmp dir#17

Merged
gusfcarvalho merged 1 commit into
mainfrom
gc-fix-setup-cache-on-tmp
Jun 3, 2026
Merged

fix: setup cache on tmp dir#17
gusfcarvalho merged 1 commit into
mainfrom
gc-fix-setup-cache-on-tmp

Conversation

@gusfcarvalho

@gusfcarvalho gusfcarvalho commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Resolved Cloud Custodian execution failures in read-only filesystem environments and restricted deployment scenarios by configuring an ephemeral cache location for the resource cache, eliminating dependency on inaccessible default cache paths.

Review Change Stack

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Copilot AI review requested due to automatic review settings June 3, 2026 11:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR adds an internal helper function custodianCachePath() that computes a writable cache location under os.TempDir(), then integrates it into the Cloud Custodian dry-run command invocation by passing the path explicitly via the --cache CLI flag. This ensures the resource cache works in read-only filesystem environments without reliance on a writable home directory.

Changes

Custodian Ephemeral Cache Path

Layer / File(s) Summary
Custodian ephemeral cache path configuration
main.go
Helper function custodianCachePath() computes a writable cache location under os.TempDir(). The dry-run custodian invocation passes this path via --cache flag instead of relying on the default ~/.cache location.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A cache that lives in /tmp and flies,
No home directory needed, oh what a prize!
Read-only roots can't slow this down,
Cloud Custodian scans the whole town!
Ephemeral paths make the temp dir proud, ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: setup cache on tmp dir' directly describes the main change: configuring Cloud Custodian's cache to use a temporary directory instead of the default location.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@main.go`:
- Around line 375-377: custodianCachePath currently returns a global temp file
which causes cross-run contention; change it to produce a per-run cache file
under the run's output directory (use req.OutputDir) instead of os.TempDir().
For example, modify custodianCachePath to accept an outputDir parameter (or
remove it and inline a new join using req.OutputDir where called) and return
filepath.Join(outputDir, "cloud-custodian.cache"); update all call sites (the
locations that call custodianCachePath() around the uses at/near
custodianCachePath and the caller at line ~507) to pass req.OutputDir so each
run gets an isolated cache file that is automatically within the run's output
directory.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c67d8807-0fd4-428a-8da2-ac48ad7d704a

📥 Commits

Reviewing files that changed from the base of the PR and between c4a8115 and 609b55e.

📒 Files selected for processing (1)
  • main.go

Comment thread main.go
Comment on lines +375 to +377
func custodianCachePath() string {
return filepath.Join(os.TempDir(), "cloud-custodian.cache")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a run-scoped cache path instead of a shared global temp file.

Line 375 and Line 507 currently force every execution to the same cache file (/tmp/cloud-custodian.cache or $TMPDIR/...). That introduces cross-run cache sharing and potential contention/stale data between checks or concurrent plugin executions. Prefer a per-execution cache file under req.OutputDir so isolation and cleanup are automatic.

Suggested diff
-func custodianCachePath() string {
-	return filepath.Join(os.TempDir(), "cloud-custodian.cache")
+func custodianCachePath(outputDir string) string {
+	return filepath.Join(outputDir, "cloud-custodian.cache")
 }
@@
-	args = append(args, "--dryrun", "-s", req.OutputDir, "--cache", custodianCachePath(), policyPath)
+	args = append(args, "--dryrun", "-s", req.OutputDir, "--cache", custodianCachePath(req.OutputDir), policyPath)

Also applies to: 507-507

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@main.go` around lines 375 - 377, custodianCachePath currently returns a
global temp file which causes cross-run contention; change it to produce a
per-run cache file under the run's output directory (use req.OutputDir) instead
of os.TempDir(). For example, modify custodianCachePath to accept an outputDir
parameter (or remove it and inline a new join using req.OutputDir where called)
and return filepath.Join(outputDir, "cloud-custodian.cache"); update all call
sites (the locations that call custodianCachePath() around the uses at/near
custodianCachePath and the caller at line ~507) to pass req.OutputDir so each
run gets an isolated cache file that is automatically within the run's output
directory.

@gusfcarvalho gusfcarvalho merged commit 5733f27 into main Jun 3, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants